Skip to content

Conversation

MovGP0
Copy link
Contributor

@MovGP0 MovGP0 commented Sep 9, 2025

This is a port of the material-color-utilities (MCU) to C#.

  • This code can be used for automatic MaterialDesign3 color scheme generation from a selected color.
  • Unit tests are ported from the dart version of the library
  • See m3.material.io: Color system for details
  • Implemented Shims to work with net462 and net8.0-windows targets

@MovGP0
Copy link
Contributor Author

MovGP0 commented Sep 10, 2025

The library creates colors according to this system:
image

@MovGP0 MovGP0 force-pushed the master branch 2 times, most recently from b71e3ee to a2f825d Compare September 12, 2025 17:22
@Keboo Keboo self-requested a review September 15, 2025 05:16
@Keboo
Copy link
Member

Keboo commented Sep 15, 2025

Wow @MovGP0 this looks really cool. It will take me some time to get it reviewed. But thank you for it.

@MovGP0
Copy link
Contributor Author

MovGP0 commented Sep 21, 2025

@Keboo I was trying to keep the implementation as close as possible to the original implementation.

However, this has also has some drawback we should consider:

  1. colors are represented as signed 32bit integers, instead of System.Windows.Color. While a conversion is easily possible, the question is if we only should use a conversion at the border of the library, or also replace int with Color for internal usage and break with the original source.
  2. the unit tests don't have a 100% coverage and have multiple asserts per test. While this makes it aligned wtih the original source code, I'd consider it bad practice to structure unit tests like this.
  3. the CS0618 ([Obsolete]) warning is on purpose, since this is how it's in the original code. The question is if this code should be removed or the warning suppressed. Currently it breaks the build.

Johann Dirry and others added 2 commits September 23, 2025 17:48
Uses collection expressions and generic `Enum.GetValues`, suppresses obsolete member warnings, and removes an unnecessary assertion.
@Keboo Keboo enabled auto-merge (squash) October 10, 2025 07:01
@Keboo
Copy link
Member

Keboo commented Oct 10, 2025

@Keboo I was trying to keep the implementation as close as possible to the original implementation.

However, this has also has some drawback we should consider:

  1. colors are represented as signed 32bit integers, instead of System.Windows.Color. While a conversion is easily possible, the question is if we only should use a conversion at the border of the library, or also replace int with Color for internal usage and break with the original source.
  2. the unit tests don't have a 100% coverage and have multiple asserts per test. While this makes it aligned wtih the original source code, I'd consider it bad practice to structure unit tests like this.
  3. the CS0618 ([Obsolete]) warning is on purpose, since this is how it's in the original code. The question is if this code should be removed or the warning suppressed. Currently it breaks the build.
  1. I think at the border of the library using Color over int will make it much more usable.
  2. I am fine with the tests having multiple asserts. Usually, the guidance I use is having tests assert on one "logical" thing, even if that means multiple assert statement. And I am happy to make exceptions in certain cases.
  3. I pushed a commit to suppress the warning inline for now.

I think I am going to set this to merge. Since it is not currently being consumed or published I think it will then allow for easier collaboration on it moving forward. I think getting the int -> Color bit will be helpful and samples in the demo app.

Updates the `Scheme_Correctness_All` method signature from `async Task` to `void`.
@Keboo Keboo added this to the 5.3.1 milestone Oct 10, 2025
@Keboo Keboo added the release notes Items are likely to be highlighted in the release notes. label Oct 10, 2025
@Keboo Keboo merged commit 8304e64 into MaterialDesignInXAML:master Oct 10, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes Items are likely to be highlighted in the release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants